Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

GH-40108: [JS] Remove SWC dependency and move types to dev deps #41274

Closed
wants to merge 2 commits into from
Closed

GH-40108: [JS] Remove SWC dependency and move types to dev deps #41274

wants to merge 2 commits into from

Conversation

domoritz
Copy link
Member

@domoritz domoritz commented Apr 18, 2024 •
edited
Loading

@domoritz domoritz requested a review from trxcllnt as a code owner April 18, 2024 03:05
@github-actions GitHub Actions
Copy link

⚠️ GitHub issue #40108 has been automatically assigned in GitHub to PR creator.

@domoritz domoritz changed the title GH-40108: [JS] Remove dependencies GH-40108: [JS] Remove SWC dependency Apr 18, 2024
@domoritz domoritz mentioned this pull request Apr 18, 2024
Open
@domoritz domoritz added this to the 17.0.0 milestone Apr 18, 2024
trxcllnt
trxcllnt requested changes Apr 18, 2024
View reviewed changes
Copy link
Contributor

@trxcllnt trxcllnt left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per the ts-node docs:

SWC uses @swc/helpers instead of tslib. If you have enabled importHelpers, you must also install @swc/helpers.

@domoritz
Copy link
Member Author

domoritz commented Apr 18, 2024 •
edited
Loading

I thought I disabled swc though so ts node shouldn't use it anymore. See https://github.com/apache/arrow/pull/41274/files#diff-bb9cf0fd13f06ac1b6668c895fdba550244bc31245ee5a43121a1609af54f067L22

@domoritz domoritz changed the title GH-40108: [JS] Remove SWC dependency GH-40108: [JS] Remove SWC dependency and move types to dev deps Apr 18, 2024
@trxcllnt
Copy link
Contributor

IIRC the reason we're using swc was that the regular ts-node compilation speed is so slow, it was timing out the integration tests.

@trxcllnt
Copy link
Contributor

trxcllnt commented Apr 19, 2024 •
edited
Loading

Specifically this commit: c6cc6c0. The CI run for the previous commit timed out after 60 minutes.

@domoritz
Copy link
Member Author

Yeah, looks like we need 45 minutes rather than 25 compared to #41260. So yeah, not a change we should make.

@domoritz
Copy link
Member Author

domoritz commented Apr 19, 2024 •
edited
Loading

Closing this for now as it seems like we need to make more major cleanups.

@domoritz domoritz closed this Apr 19, 2024
@domoritz domoritz deleted the dom/40108 branch April 19, 2024 02:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@trxcllnt trxcllnt trxcllnt requested changes

Assignees
No one assigned
Labels
awaiting committer review Awaiting committer review Component: JavaScript
Projects
None yet
Milestone
17.0.0
Development

Successfully merging this pull request may close these issues.
2 participants
@domoritz @trxcllnt